Hello @ohruka , thanks for the PR! There are a lot of technical details to change, including the rpm build failure:

install: cannot stat '/builddir/build/SOURCES/src/systemd/cockpit-machines-logging.service': No such file or directory

But I'd like to discuss the high-level strategy first. They were already discussed in https://issues.redhat.com/browse/RHEL-22717 , not sure if you followed that?

If that suits your specific use case, that certainly works, and you are welcome to use it in your deployment. Especially if you drop the extra shell script and call busctl right from the unit, it becomes a single file to install, so it can be done easily with Ansible or a README or similar.

I have a lot of reservations about this approach; it's not something which we'd put into the default package, as there are too many conceptual problems with it as a generic solution:

• It's missing a lot of cockpit-machines actions which are not happening via D-Bus (virt-install, virt-xml, and in one case even virsh), as a lot of functionality isn't offered by libvirt-dbus
• It creates a firehose of information in the log, most of which isn't interesting (in particular not the "read-only" bits)
• It's not a pattern which works (or we intend to work) for other cockpit pages
• It's not a generic "what happens to my VMs" audit log, and shipping it by default would raise expectations that we can't fulfill.

My proposal is to replace this with a README update that explains how to debug/follow what c-machines is doing. That can explain busctl monitor --match 'interface=org.libvirt.Domain' for the D-Bus part. I'd go as far as shipping that unit in /usr/share/doc/ or in the README, with the caveat that it's only a third of the information.

There is also window.debugging = "machines" for the other half that calls virt-install and such, but this is much harder to use: it's way too verbose, and most of it isn't useful for your use case, and it requires using the browser console which is less discoverable for admins than the systemd journal. To mitigate that, we could teach the install_machine.py script to log the virt-install command to the journal. Would that help you? That still misses calls to virt-xml and virsh, but at least is a very big and interesting part of what c-machines does.

Hello @martinpitt , Thank you for your reply and the many comments and suggestions.

This PR is related to RHEL-22717.

I have made the following changes based on the comments in the Jira ticket to enable this feature to be included in the package.

• Changed the log output destination from custom log file (/var/log/libvirt/*) to journal.
• Only collect logs from interface=org.libvirt.Domain.
• The feature does not work by default (enable only when necessary).

It's missing a lot of cockpit-machines actions which are not happening via D-Bus (virt-install, virt-xml, and in one case even virsh), as a lot of functionality isn't offered by libvirt-dbus

I have not added this because it can be checked with existing logs.
virt-install and virt-xml output logs to their own log files when executed.
virt-install outputs to ~/.cache/virt-manager/virt-install.log, virt-xml outputs to ~/.cache/virt-manager/virt-xml.log.
I am not collecting logs to avoid duplicate information.

It creates a firehose of information in the log, most of which isn't interesting (in particular not the "read-only" bits)

It is true that both useful and useless information is output.
On the other hand, by narrowing down the output to interface=org.libvirt.Domain, basically only records of operations in the machines environment should be output.

It's not a pattern which works (or we intend to work) for other cockpit pages

This is as you pointed out. Creating a new operation log function for the entire cockpit would be a huge undertaking, so I am only creating it for the cockpit-machines part to satisfy the function that was in virt-manager.

Currently, I believe that cockpit itself does not have logs that record "when and what operations were performed".
If such a function to record across the board is posted as a patch, is there any room for discussion to incorporate it as a default feature of cockpit?

My proposal

Thank you for the suggestion.
The reason why I am creating a systemd service that explicitly runs busctl in this patch is that I want to allow users to enable specific features and collect logs.
If you run busctl directly, you will need some additional skills to collect logs continuously.

Is there a possibility that this patch will be incorporated as a default feature by making these corrections?
Or are there concerns about incorporating the cockpit-machines log function itself as a default feature?

Currently, I believe that cockpit itself does not have logs that record "when and what operations were performed".

Some services do, many in the form of "audit" logs, others in the normal journal. For example, when you stop a service on the "Services" page, that gets an entry in the journal. Same for logging in and out of cockpit (that's the normal PAM logs for starting/stopping a user session. Or if you e.g. stop/start/change a network interface, you get NetworkManger journal logs. Creating a new user also gets logged, but e.g. deleting a Linux user doesn't. But we consider that a bug in PAM or deluser -- in our view it's really not Cockpit's responsibility to do this logging, as operations such as deluser are also run interactively from admins, or from Ansible, shell scripts, or any number of other places.

If such a function to record across the board is posted as a patch, is there any room for discussion to incorporate it as a default feature of cockpit?

I'm afraid no, for the reasons above. Doing things in Cockpit, Terminal, Ansible etc. should all be interchangeable and be possible to do side by side. Logging such actions from Cockpit is simply the wrong layer. Logging actions needs to be done by the API we talk to (D-Bus service, Linux subsystem, administrative CLI program, etc.).

This is why I am very concerned about adding this to c-machines: It sets a bad example and raises expectations which we have to refuse in other places. It needs to be absolutely crystal clear that such a log is neither a security proof nor reliable -- it is a debugging tool, nothing else.

by narrowing down the output to interface=org.libvirt.Domain, basically only records of operations in the machines environment should be output

Yes, that's a given, but my "firehose" comment really applied to that restriction. It has all the read operations such as GetXMLDesc, GetStats, ListDomainSnapshots etc, or signals. So to be usable, this would need some post processing and knowledge which API calls are actually changing things.

That's why I'm not a fan of a plain busctl, and letting this run in a permanent service -- it'll accumulate gigabytes of logs which clutter the journal and make it much less useful for other tasks (admins look at journalctl very often) -- for that reason I also retract my request to log this into the journal -- it should really go into a separate file with configured log rotation. Sorry about the back and forth.

Thank you for your comments.
Based on the feedback we received, we've decided not to add the client operation logging feature to cockpit-machines.
Therefore, I'm closing this pull request.